Skip to content

Add Claude Desktop as an LLM gateway client#5712

Open
jerm-dro wants to merge 4 commits into
mainfrom
feat/llm-claude-desktop
Open

Add Claude Desktop as an LLM gateway client#5712
jerm-dro wants to merge 4 commits into
mainfrom
feat/llm-claude-desktop

Conversation

@jerm-dro

@jerm-dro jerm-dro commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Summary

thv llm setup routes Claude Code and several editors through an OIDC-protected LLM gateway, but not Claude Desktop. Claude Desktop exposes a different configuration surface — its "third-party inference" (Cowork 3P) model — which uses a configLibrary config document plus a _meta.json selector and a credential-helper executable, none of which fit the existing JSON-key-patching path.

This adds a credential-helper client mode that reuses thv llm token:

  • New ClaudeDesktop client (LLMGatewayMode: "credential-helper"). Writes a <uuid>.json config document (inferenceProvider: gateway, base URL, inferenceCredentialKind: helper-script, optional inferenceModels) and merges a _meta.json selector, preserving any user-owned entries. Generates a no-arg shim that execs thv llm token (browser only in the interactive helper context; silent otherwise).
  • Detection via Claude Desktop's app-support directory (a GUI app, not on $PATH); warns when a managed-preferences profile would override the local config; tells the user to relaunch (config is read only at launch).
  • Teardown removes the entry, config document, and shim, and clears appliedId only when it pointed at ToolHive's config.
  • New --models flag feeds inferenceModels. The key is omitted when empty so Claude Desktop can fall back to gateway-side model discovery once the gateway serves it (tracked in stacklok/stacklok-enterprise-platform#2077 — the gateway's /anthropic/v1/models currently 404s, so --models is the interim path).

Type of change

  • New feature

Test plan

  • Unit tests (task test) — new writer/_meta merge/shim/teardown/detection tests in pkg/client
  • E2E tests (task test-e2e) — new claude-desktop setup→verify→teardown spec in the all-client matrix
  • Linting (task lint-fix)
  • Manual testing (describe below)

Manual testing: Ran thv llm setup --client claude-desktop --models … against the staging gateway on a real macOS Claude Desktop install. Verified the written configLibrary/<uuid>.json, _meta.json selector, and generated shim match Claude Desktop's own schema; relaunched the app and confirmed the "ToolHive Gateway" config applied and live inference worked on all models. Ran thv llm teardown claude-desktop and confirmed the entry, config document, and shim were removed and appliedId cleared.

Special notes for reviewers

  • Open item — Developer Mode: setup writes the configLibrary files directly (no in-app UI), so setup itself doesn't need Claude Desktop's Developer Mode. Whether the app applies the local config at launch with Developer Mode off is not yet confirmed — the live test above was run with Developer Mode on. Worth a quick verification before we document "no Developer Mode required."
  • Windows is not supported yet: the credential-helper shim is a POSIX /bin/sh script, consistent with the rest of the LLM-gateway token-helper feature (buildTokenHelperCommand is already POSIX-only). Setup returns a clear error on Windows.
  • The credential-helper document/selector model is deliberately kept out of the LLMGatewayKeys JSON-pointer machinery — it's a separate writer (pkg/client/llm_gateway_credential_helper.go) dispatched on LLMGatewayMode == "credential-helper".

Large PR Justification

This is a single cohesive feature — Claude Desktop as an LLM-gateway client — where most of the line count is not hand-written business logic and does not split cleanly below the threshold:

  • Roughly half is one self-contained new file (pkg/client/llm_gateway_credential_helper.go) plus its unit tests. The credential-helper writer is one atomic unit (write config document + _meta.json selector + shim, and revert them together); splitting it mid-writer would leave a non-functional intermediate state.
  • Tests and docs are a large share — writer unit tests, an end-to-end setup→verify→teardown spec, and regenerated CLI docs — which ToolHive's own PR-size guidance excludes from the limit.
  • Review-driven hardening on this branch added further mostly test-only and small production changes (path-traversal validation, file-lock scoping, malformed-_meta.json handling) in response to reviewer feedback.

The production surface is ~500 lines across 6 files; the remainder is tests and docs. A split was considered (a pkg/client capability PR vs. a pkg/llm + CLI wiring PR), but the two halves are only meaningful together and the feature was verified end-to-end as one unit.

🤖 Generated with Claude Code

thv llm setup can route Claude Code and several editors through an
OIDC-protected LLM gateway, but not Claude Desktop. Claude Desktop
exposes a different configuration surface — its "third-party inference"
(Cowork 3P) model — which uses a configLibrary document plus a
_meta.json selector and a credential-helper executable, none of which
fit the existing JSON-key-patching path.

Add a credential-helper client mode that reuses "thv llm token":

- New ClaudeDesktop client with LLMGatewayMode "credential-helper".
  Writes a <uuid>.json config document (inferenceProvider/gateway URL/
  helper-script kind/models) and merges a _meta.json selector, preserving
  any user-owned entries. Generates a no-arg shim that execs
  "thv llm token" (browser only in the interactive helper context).
- Detect Claude Desktop by its app-support directory (a GUI app, not on
  $PATH); warn when a managed-preferences profile would override the
  local config; tell the user to relaunch (config is read at launch).
- Teardown removes the entry, config document, and shim, and clears
  appliedId only when it pointed at ToolHive's config.
- New --models flag feeds inferenceModels; the key is omitted when empty
  so the app can fall back to gateway model discovery once available.

Verified end-to-end against a real Claude Desktop install (config
applied, live inference on all models) plus unit and e2e coverage.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR extends thv llm setup to support configuring Claude Desktop as an LLM gateway client using Claude Desktop’s third-party inference credential-helper configuration model (configLibrary <id>.json + _meta.json selector + helper executable), rather than the existing JSON-pointer patching approach used by other tools. It introduces a dedicated credential-helper writer, adds a --models flag to optionally populate inferenceModels, and updates docs/tests to cover setup/teardown and detection.

Changes:

  • Add a new Claude Desktop LLM gateway client mode (credential-helper) with dedicated configLibrary writer, shim generation, detection, and teardown.
  • Thread a new --models flag through CLI → setup pipeline and persist it into credential-helper configs (omitted when empty).
  • Add unit + E2E coverage for Claude Desktop setup/verify/teardown and update docs/CLI help text.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
test/e2e/cli_llm_all_clients_test.go Adds an E2E scenario covering Claude Desktop credential-helper setup, verification, and teardown.
pkg/llmgateway/config.go Introduces Claude Desktop helper TTL/timeout constants and adds Models to ApplyConfig.
pkg/llm/setup.go Threads models into setup, applies Anthropic base URL logic to credential-helper mode, and adds credential-helper warnings.
pkg/llm/setup_test.go Updates setup tests for the new Setup signature and GatewayManager interface.
pkg/client/llm_gateway.go Dispatches Configure/Revert to the credential-helper writer, adds managed-profile detection, and improves GUI app detection paths.
pkg/client/llm_gateway_credential_helper.go New credential-helper implementation: writes shim, config doc, and _meta.json selector; supports teardown and managed-profile detection.
pkg/client/llm_gateway_credential_helper_test.go Unit tests for config doc writing, _meta.json merge behavior, shim creation, idempotency, and teardown behavior.
pkg/client/config.go Registers claude-desktop client integration metadata (paths, detection, managed profile domain, credential-helper capability).
docs/cli/thv_llm_setup.md Documents Claude Desktop support and the new --models flag semantics.
cmd/thv/app/llm.go Adds --models flag and passes it into llm.Setup; updates command long description to include Claude Desktop.
cmd/thv/app/llm_test.go Updates CLI tests for the new runLLMSetup signature.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread pkg/client/llm_gateway_credential_helper.go Outdated
Comment thread pkg/client/llm_gateway_credential_helper.go Outdated
Comment thread pkg/llm/setup.go
@jerm-dro

jerm-dro commented Jul 2, 2026

Copy link
Copy Markdown
Contributor Author

Developer Mode (open): Setup writes the configLibrary files directly, no UI. Confirmed live inference works with the app's allowDevTools flag off — but that flag is not the Developer Mode gate (the Developer menu persisted; it only controls the DevTools inspector), and the real Developer Mode toggle isn't a discoverable config flag, so we couldn't test a true Developer-Mode-off state. Recommend verifying on a clean/never-enabled install before documenting Developer Mode as not-required.

- revertCredentialHelper: de-reference _meta.json and delete the config
  document before removing the shim, so a mid-revert failure never leaves
  Claude Desktop pointing at a config that references a missing helper.
  Treat an empty configPath as a no-op (do not touch the shim when we
  cannot confirm the selector no longer references it).
- configureCredentialHelper: best-effort cleanup of the config document and
  a freshly-created shim when setup fails, so a failure leaves no partial
  state. Skip shim removal when an earlier setup already created it (setup
  is idempotent).
- Make the relaunch note platform-neutral (drop macOS-specific "Cmd-Q").

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/L Large PR: 600-999 lines changed and removed size/L Large PR: 600-999 lines changed labels Jul 2, 2026
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 72.33010% with 57 lines in your changes missing coverage. Please review.
✅ Project coverage is 70.64%. Comparing base (2aca563) to head (16bc233).
⚠️ Report is 18 commits behind head on main.

Files with missing lines Patch % Lines
pkg/client/llm_gateway_credential_helper.go 70.28% 30 Missing and 22 partials ⚠️
pkg/client/llm_gateway.go 58.33% 4 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #5712      +/-   ##
==========================================
+ Coverage   70.63%   70.64%   +0.01%     
==========================================
  Files         667      683      +16     
  Lines       67607    69055    +1448     
==========================================
+ Hits        47752    48786    +1034     
- Misses      16399    16703     +304     
- Partials     3456     3566     +110     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@jhrozek jhrozek left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A few things I ran into going through the Claude Desktop credential-helper code — mostly small, one worth a closer look before merging.

Comment thread pkg/client/llm_gateway_credential_helper.go Outdated
Comment thread pkg/client/llm_gateway_credential_helper.go Outdated
Comment thread pkg/client/llm_gateway_credential_helper.go Outdated
Comment thread pkg/client/llm_gateway_credential_helper.go Outdated
Comment thread pkg/client/config.go Outdated
Comment thread pkg/client/llm_gateway_credential_helper.go
Comment thread pkg/llm/setup.go
Comment thread pkg/client/llm_gateway_credential_helper.go
aponcedeleonch
aponcedeleonch previously approved these changes Jul 3, 2026

@aponcedeleonch aponcedeleonch left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If _meta.json is valid JSON but entries isn't an array (say an object, or absent-and-later-set-wrong), configure then does meta["entries"] = append(nil, ourEntry) and silently drops whatever was there. That quietly breaks the "preserve every entry ToolHive does not own" contract the writer is built around. Fully-malformed JSON already aborts safely on the parse error, so this is the narrow "valid JSON, wrong shape" case.

Same input class shows up in metaEntryID: if a name-matching entry has a non-string id, it returns "", and configure then generates a fresh UUID and appends a second "ToolHive Gateway" entry, which defeats the idempotency the stable name is there to provide.

Both are low-likelihood, but since the whole point of reading into a generic map is to not stomp on user data, I'd lean toward bailing with an error when entries is present and not a []any, rather than overwriting. A small test with a non-array entries would lock it in.

- Validate the config id read from _meta.json before joining it into a
  path (reject non-bare-filename / ".." values), so a corrupted or
  hand-edited selector cannot make setup write outside configLibrary.
- Move shim create/remove inside the _meta.json file lock so concurrent
  setup/teardown runs cannot interleave (one run's failure-cleanup
  deleting a shim another run's committed config depends on).
- Reuse resolveApplyConfigField for the AnthropicBaseURL/GatewayURL
  fallback instead of duplicating it.
- Warn on a non-string appliedId in _meta.json instead of silently
  treating it as absent.
- Drop the redundant LLMCredentialHelper bool; dispatch everywhere on
  LLMGatewayMode == "credential-helper" via a shared constant.
- Make the managed-profile check unit-testable (injectable root) and
  cover it, plus warnCredentialHelperTools and the setup-failure cleanup
  path, with tests.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Large PR Detected

This PR exceeds 1000 lines of changes and requires justification before it can be reviewed.

How to unblock this PR:

Add a section to your PR description with the following format:

## Large PR Justification

[Explain why this PR must be large, such as:]
- Generated code that cannot be split
- Large refactoring that must be atomic
- Multiple related changes that would break if separated
- Migration or data transformation

Alternative:

Consider splitting this PR into smaller, focused changes (< 1000 lines each) for easier review and reduced risk.

See our Contributing Guidelines for more details.


This review will be automatically dismissed once you add the justification section.

@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/L Large PR: 600-999 lines changed labels Jul 6, 2026
- readClaudeDesktopMeta now errors when _meta.json is valid JSON but its
  "entries" is not an array, instead of silently treating it as empty and
  overwriting the user's saved configurations on the next write.
- Upsert the ToolHive entry by name so a malformed or stale same-name
  entry (e.g. a non-string id) is corrected in place rather than leaving a
  duplicate "ToolHive Gateway" entry, preserving idempotency.
- Tests for the non-array-entries error and the non-string-id no-duplicate
  cases.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@github-actions github-actions Bot added size/XL Extra large PR: 1000+ lines changed and removed size/XL Extra large PR: 1000+ lines changed labels Jul 6, 2026
@jerm-dro

jerm-dro commented Jul 6, 2026

Copy link
Copy Markdown
Contributor Author

@aponcedeleonch addressed in 16bc233 (thanks — good catch on the "valid JSON, wrong shape" case):

  • readClaudeDesktopMeta now errors when _meta.json is valid JSON but entries is present and not an array, instead of silently treating it as empty and overwriting the user's saved configurations on the next write.
  • The ToolHive entry is now upserted by name, so a malformed or stale same-name entry (e.g. a non-string id) is corrected in place rather than leaving a duplicate "ToolHive Gateway" entry — preserving the idempotency the stable name provides.
  • Tests: TestConfigureCredentialHelper_RejectsNonArrayEntries and TestConfigureCredentialHelper_NonStringIDDoesNotDuplicate.

@github-actions github-actions Bot removed the size/XL Extra large PR: 1000+ lines changed label Jul 6, 2026
@github-actions github-actions Bot added the size/XL Extra large PR: 1000+ lines changed label Jul 6, 2026
@github-actions github-actions Bot dismissed their stale review July 6, 2026 17:39

Large PR justification has been provided. Thank you!

@github-actions

github-actions Bot commented Jul 6, 2026

Copy link
Copy Markdown
Contributor

✅ Large PR justification has been provided. The size review has been dismissed and this PR can now proceed with normal review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Extra large PR: 1000+ lines changed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants